-
Notifications
You must be signed in to change notification settings - Fork 4
chore: Rename jupyter command prefix to deepnote #172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai full review
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR performs a repository-wide rename from Jupyter-prefixed identifiers to Deepnote-prefixed identifiers. Changes include command IDs, VS Code context keys, configuration namespaces, localization keys, widget MIME types, default comm target, static tool names, viewType strings, and many tests. No control-flow, error-handling, or algorithmic behavior was altered; some exported string constants and view-type literals were updated. A few literal typos were preserved or moved into the Deepnote namespace. Sequence Diagram(s)Skipped — the changes are broad identifier and string renames without new or modified control flow to visualize. Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/test/smoke/datascience.smoke.test.ts (1)
120-151: Command renames correct; consider cleaning unreachable code.The command renames from
jupyter.createnewinteractivetodeepnote.createnewinteractiveare correct. However, all code afterreturn this.skip()at line 121 is unreachable. Consider removing or moving it to a comment block if kept for reference.Apply this diff to clean up unreachable code:
test('Interactive window should always pick up current active interpreter', async function () { return this.skip(); // See https://github.com/microsoft/vscode-jupyter/issues/5478 - - // Make an interactive window - await vscode.commands.executeCommand<void>('deepnote.createnewinteractive'); - assert.ok(vscode.workspace.notebookDocuments.length === 1, 'Unexpected number of notebook documents created'); - // const currentWindow = provider.windows[0]; - // const interpreterForCurrentWindow = currentWindow.notebook?.getMatchingInterpreter(); - // assert.ok(interpreterForCurrentWindow !== undefined, 'Unable to get matching interpreter for current window'); - - // Now change active interpreter - await waitForCondition( - async () => getCachedEnvironments().length > 0, - 15_000, - 'Waiting for interpreters to be discovered' - ); - - assert.ok(getCachedEnvironments().length > 1, 'Not enough interpreters to run interactive window smoke test'); - // const differentInterpreter = allInterpreters.find((interpreter) => interpreter !== interpreterForCurrentWindow); - // await vscode.commands.executeCommand<void>('python.setInterpreter', differentInterpreter); // Requires change to Python extension - - // // Now make another interactive window and confirm it's using the newly selected interpreter - // await vscode.commands.executeCommand<void>('deepnote.createnewinteractive'); - // assert.ok(provider.windows.length === 2, 'Unexpected number of interactive windows created'); - // const newWindow = provider.windows.find((window) => window !== currentWindow); - // const interpreterForNewWindow = newWindow?.notebook?.getMatchingInterpreter(); - // assert.ok(interpreterForNewWindow !== undefined, 'Unable to get matching interpreter for current window'); - // assert.ok( - // interpreterForNewWindow === differentInterpreter, - // 'Interactive window not created with newly selected interpreter' - // ); });src/interactive-window/editor-integration/cellRangeCache.unit.test.ts (1)
60-73: Renaming correct, but test logic inconsistent.The context key renaming from
jupyter.*todeepnote.*is correct. However, the test name claims context keys "will set...to false" but lines 71-72 verify they're set totrue. Pre-existing issue worth fixing.src/standalone/intellisense/kernelCompletionProvider.ts (2)
562-562: Critical bug: Configuration namespace not updated.The configuration namespace is still
'jupyter'but should be'deepnote'to match the renamed setting keys. This will causeenableKernelCompletionsto be read from the wrong namespace.Apply this diff:
function isKernelCompletionEnabled(resource: Resource) { - return workspace.getConfiguration('jupyter', resource).get<boolean>('enableKernelCompletions', false); + return workspace.getConfiguration('deepnote', resource).get<boolean>('enableKernelCompletions', false); }
565-590: Critical bug: Configuration namespace not updated.The configuration namespace is still
'jupyter'but should be'deepnote'to match the renamed setting keys. This will causecompletionTriggerCharactersto be read from the wrong namespace.Apply this diff:
function getCompletionTriggerCharacter(kernel: IKernel) { const triggerCharacters = workspace - .getConfiguration('jupyter', kernel.notebook.uri) + .getConfiguration('deepnote', kernel.notebook.uri) .get<Record<string, string[]>>('completionTriggerCharacters');package.nls.json (1)
1-288: Fix typos in context condition strings (src/platform/common/constants.ts, lines 299–300)The flagged typos exist and require correction:
- Line 299:
'deepnote.ispythonorinteractiveeactive'→'deepnote.ispythonorinteractiveactive'- Line 300:
'deepnote.ispythonorinteractiveornativeeactive'→'deepnote.ispythonorinteractiveornativeactive'Both strings have an erroneous extra 'e' before 'active'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (54)
.github/instructions/ipywidgets.instructions.md(25 hunks)api/README.md(1 hunks)build/preDebugWebTest.js(1 hunks)package.nls.json(3 hunks)src/commands.ts(3 hunks)src/extension.common.ts(1 hunks)src/extension.node.ts(1 hunks)src/extension.web.ts(1 hunks)src/interactive-window/commands/commandRegistry.ts(1 hunks)src/interactive-window/editor-integration/cellRangeCache.ts(2 hunks)src/interactive-window/editor-integration/cellRangeCache.unit.test.ts(3 hunks)src/kernels/errors/kernelErrorHandler.ts(1 hunks)src/kernels/errors/kernelErrorHandler.unit.test.ts(3 hunks)src/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.node.ts(1 hunks)src/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.unit.test.ts(2 hunks)src/notebooks/controllers/commands/installPythonControllerCommands.ts(1 hunks)src/notebooks/controllers/ipywidgets/message/ipyWidgetMessageDispatcher.ts(1 hunks)src/notebooks/controllers/kernelSource/kernelSourceCommandHandler.ts(6 hunks)src/notebooks/outputs/linkProvider.ts(1 hunks)src/platform/common/application/commands/reloadCommand.node.ts(1 hunks)src/platform/common/application/commands/reloadCommand.unit.test.ts(1 hunks)src/platform/common/application/commands/runInDedicatedExtensionHost.node.ts(1 hunks)src/platform/common/cache.ts(1 hunks)src/platform/common/constants.ts(6 hunks)src/platform/common/utils/localize.ts(5 hunks)src/platform/errors/errorUtils.ts(1 hunks)src/platform/interpreter/filter/completionProvider.node.ts(1 hunks)src/platform/interpreter/filter/filterService.ts(1 hunks)src/platform/interpreter/filter/uiDeprecationHandler.ts(1 hunks)src/platform/logging/index.ts(1 hunks)src/platform/webviews/types.ts(1 hunks)src/platform/webviews/webviewHost.ts(1 hunks)src/standalone/api/kernels/accessManagement.ts(1 hunks)src/standalone/chat/configureNotebook.node.ts(1 hunks)src/standalone/chat/extension.node.ts(2 hunks)src/standalone/chat/installPackageTool.node.ts(1 hunks)src/standalone/chat/listPackageTool.node.ts(1 hunks)src/standalone/chat/restartKernelTool.node.ts(2 hunks)src/standalone/executionAnalysis/extension.ts(5 hunks)src/standalone/executionAnalysis/symbols.ts(1 hunks)src/standalone/intellisense/kernelCompletionProvider.ts(3 hunks)src/standalone/userJupyterServer/serverSelectorForTests.ts(1 hunks)src/test/.vscode/settings.json(1 hunks)src/test/common.node.ts(1 hunks)src/test/common.web.ts(1 hunks)src/test/datascience/.vscode/settings.json(1 hunks)src/test/datascience/interactiveDebugging.vscode.test.ts(1 hunks)src/test/datascience/interactiveWindow.vscode.common.test.ts(1 hunks)src/test/datascience/notebook/exportFull.vscode.test.ts(5 hunks)src/test/index.node.ts(1 hunks)src/test/smoke/datascience.smoke.test.ts(3 hunks)src/test/standardTest.node.ts(2 hunks)src/test/web/index.ts(2 hunks)src/webviews/extension-side/variablesView/variableViewProvider.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (19)
src/kernels/errors/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Use specific error types (e.g., KernelDeadError, KernelConnectionTimeoutError, KernelInterruptTimeoutError, JupyterInvalidKernelError, KernelDependencyError) for failure cases
Files:
src/kernels/errors/kernelErrorHandler.tssrc/kernels/errors/kernelErrorHandler.unit.test.ts
src/kernels/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations
Files:
src/kernels/errors/kernelErrorHandler.tssrc/kernels/errors/kernelErrorHandler.unit.test.tssrc/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.unit.test.tssrc/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.node.ts
src/standalone/chat/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/standalone.instructions.md)
src/standalone/chat/**/*.ts: Chat tool classes must extend BaseTool
Chat tools must implement prepareInvocationImpl() and invokeImpl() methods
Chat tools should declare and inject appropriate service dependencies
Chat tools must handle kernel availability and obtain user confirmation where required (e.g., restarts)
Files:
src/standalone/chat/configureNotebook.node.tssrc/standalone/chat/listPackageTool.node.tssrc/standalone/chat/installPackageTool.node.tssrc/standalone/chat/restartKernelTool.node.tssrc/standalone/chat/extension.node.ts
src/standalone/**/*.node.ts
📄 CodeRabbit inference engine (.github/instructions/standalone.instructions.md)
Implement Node-only features (filesystem, process spawning, native deps like ZeroMQ) in *.node.ts files
Files:
src/standalone/chat/configureNotebook.node.tssrc/standalone/chat/listPackageTool.node.tssrc/standalone/chat/installPackageTool.node.tssrc/standalone/chat/restartKernelTool.node.tssrc/standalone/chat/extension.node.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined
thenproperty to avoid hanging tests
Files:
src/test/datascience/interactiveWindow.vscode.common.test.tssrc/platform/common/application/commands/reloadCommand.unit.test.tssrc/kernels/errors/kernelErrorHandler.unit.test.tssrc/test/datascience/notebook/exportFull.vscode.test.tssrc/interactive-window/editor-integration/cellRangeCache.unit.test.tssrc/test/smoke/datascience.smoke.test.tssrc/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.unit.test.tssrc/test/datascience/interactiveDebugging.vscode.test.ts
src/interactive-window/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/interactiveWindow.instructions.md)
src/interactive-window/**/*.ts: Consider platform differences (Desktop/Node vs Web) for features and fallbacks when implementing functionality
Use dependency injection to keep components testable
Respect CancellationToken in async operations and support cancellation in long-running tasks
Use EventEmitter for event-driven updates and ensure proper disposal/cleanup of listeners to prevent leaks
Monitor and track promises/async operations to ensure proper cleanup and avoid dangling work on dispose
Add unit and integration tests for new features and modifications; include performance and cross-platform test coverage
Files:
src/interactive-window/editor-integration/cellRangeCache.tssrc/interactive-window/commands/commandRegistry.tssrc/interactive-window/editor-integration/cellRangeCache.unit.test.ts
src/interactive-window/editor-integration/**
📄 CodeRabbit inference engine (.github/instructions/interactiveWindow.instructions.md)
Verify cell detection across marker formats and ensure accurate source mapping when editor integration changes
Files:
src/interactive-window/editor-integration/cellRangeCache.tssrc/interactive-window/editor-integration/cellRangeCache.unit.test.ts
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/interpreter/filter/completionProvider.node.tssrc/platform/common/application/commands/reloadCommand.unit.test.tssrc/platform/logging/index.tssrc/platform/common/application/commands/reloadCommand.node.tssrc/platform/common/cache.tssrc/platform/webviews/types.tssrc/platform/interpreter/filter/filterService.tssrc/platform/common/application/commands/runInDedicatedExtensionHost.node.tssrc/platform/webviews/webviewHost.tssrc/platform/interpreter/filter/uiDeprecationHandler.tssrc/platform/errors/errorUtils.tssrc/platform/common/utils/localize.tssrc/platform/common/constants.ts
src/platform/**/*.node.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.node.ts: Provide Node.js-specific implementations in .node.ts files (e.g., FileSystem with fs-extra, PlatformService with OS detection)
Desktop implementations may use Node capabilities (fs-extra, process spawning, native modules) consistent with their responsibilities
Files:
src/platform/interpreter/filter/completionProvider.node.tssrc/platform/common/application/commands/reloadCommand.node.tssrc/platform/common/application/commands/runInDedicatedExtensionHost.node.ts
src/platform/common/application/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
Wrap VS Code APIs behind application service interfaces (e.g., IWorkspaceService, IDebugService, IApplicationEnvironment, IEncryptedStorage)
Files:
src/platform/common/application/commands/reloadCommand.unit.test.tssrc/platform/common/application/commands/reloadCommand.node.tssrc/platform/common/application/commands/runInDedicatedExtensionHost.node.ts
src/platform/logging/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Files:
src/platform/logging/index.ts
src/standalone/chat/installPackageTool.node.ts
📄 CodeRabbit inference engine (.github/instructions/standalone.instructions.md)
The package installation chat tool must be Node-only (installPackageTool.node.ts) due to process/package manager usage
Files:
src/standalone/chat/installPackageTool.node.ts
src/kernels/jupyter/interpreter/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
src/kernels/jupyter/interpreter/**/*.ts: Validate required Python packages (jupyter, ipykernel, etc.) and prompt for installation when missing
Verify nbconvert availability for export features, handling platform-specific requirements
Files:
src/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.unit.test.tssrc/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.node.ts
src/kernels/jupyter/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
src/kernels/jupyter/**/*.ts: Avoid blocking operations; use async/await consistently for asynchronous flows
Avoid shared global state for services; prefer instance-scoped or injected dependencies
Files:
src/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.unit.test.tssrc/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.node.ts
src/standalone/chat/extension.node.ts
📄 CodeRabbit inference engine (.github/instructions/standalone.instructions.md)
Register chat tools in chat/extension.node.ts via vscode.lm.registerTool(...) and add the disposables to context.subscriptions
Files:
src/standalone/chat/extension.node.ts
src/notebooks/controllers/ipywidgets/message/ipyWidgetMessageDispatcher.ts
📄 CodeRabbit inference engine (.github/instructions/ipywidgets.instructions.md)
src/notebooks/controllers/ipywidgets/message/ipyWidgetMessageDispatcher.ts: Use @jupyterlab/services serialization/deserialization for all kernel messages and handle them via KernelMessage type guards in IPyWidgetMessageDispatcher
Mirror execute_request messages to the webview before sending to the real kernel when widgets are active to keep proxy kernel futures in sync
Register comm targets via kernel.registerCommTarget and re-register them after kernel restarts; maintain pending/registered target tracking
Intercept kernel socket send/receive via hooks and add/remove them on socket changes to ensure correct forwarding to the webview
Filter and only deserialize messages likely to contain widget data (e.g., comm_open/comm_msg/comm_close, widget mimetype) to reduce overhead
For Output widget state updates, block processing until the webview acknowledges handling to avoid race conditions
On kernel restarts, clear pending state, resolve waiting promises, and re-establish hooks and comm targets
Files:
src/notebooks/controllers/ipywidgets/message/ipyWidgetMessageDispatcher.ts
src/notebooks/controllers/kernelSource/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/notebooks.instructions.md)
src/notebooks/controllers/kernelSource/**/*.ts: For new kernel sources, implement ILocalNotebookKernelSourceSelector (or similar)
Create a UI provider for kernel selection for the new kernel source
Register the new kernel source in KernelSourceCommandHandler
Files:
src/notebooks/controllers/kernelSource/kernelSourceCommandHandler.ts
src/standalone/api/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/standalone.instructions.md)
Define new API endpoint interfaces within the appropriate subdirectory under src/standalone/api/
Files:
src/standalone/api/kernels/accessManagement.ts
src/kernels/jupyter/**/*.@(node|web).ts
📄 CodeRabbit inference engine (.github/instructions/kernel-jupyter.instructions.md)
Provide platform-specific implementations using .node.ts and .web.ts for components that differ between Node and Web (e.g., HTTP requests, server features)
Files:
src/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.node.ts
🪛 Biome (2.1.2)
src/test/smoke/datascience.smoke.test.ts
[error] 124-137: This code will never be reached ...
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🔇 Additional comments (62)
src/test/smoke/datascience.smoke.test.ts (1)
67-67: Command rename in commented test is correct.The change from
jupyter.runallcellstodeepnote.runallcellsaligns with the PR objective. No runtime impact.src/test/datascience/interactiveWindow.vscode.common.test.ts (2)
143-143: Command properly registered and no lingering old references.The new
deepnote.restartkernelcommand is defined insrc/platform/common/constants.tsand the test usage is correct. The refactoring is complete.
136-136: Command registration verified.
deepnote.interactive.clearAllCellsis registered insrc/platform/common/constants.ts:265and correctly used in the test. No missed old references.src/interactive-window/editor-integration/cellRangeCache.unit.test.ts (2)
56-57: Renaming looks correct.Context keys updated from
jupyter.*todeepnote.*consistently with PR objectives.
86-87: Renaming looks correct.Context keys updated from
jupyter.*todeepnote.*consistently with PR objectives.src/standalone/chat/listPackageTool.node.ts (1)
20-20: LGTM: Tool name updated to Deepnote branding.Straightforward rename with no functional impact.
src/standalone/chat/installPackageTool.node.ts (1)
19-19: LGTM: Tool name updated to Deepnote branding.Straightforward rename. The invocation at line 89 correctly uses
RestartKernelTool.toolName, so it will automatically reference the updated restart tool name.src/standalone/chat/configureNotebook.node.ts (1)
32-32: LGTM: Tool name updated to Deepnote branding.Clean rename. All tool invocations (lines 55, 89, 133) use static property references, ensuring consistency.
src/standalone/chat/restartKernelTool.node.ts (2)
18-18: LGTM: Tool name updated to Deepnote branding.Clean rename.
47-47: LGTM: String reference updated consistently.Verified no remaining hardcoded references to old tool names in non-test code.
src/platform/errors/errorUtils.ts (1)
566-566: Telemetry tag renamed correctly—no codebase issues found.The change from
'jupyter.startup.failure'to'deepnote.startup.failure'is isolated, complete, and properly aligned with the rebranding effort. No orphaned references to the old tag exist in the codebase.api/README.md (1)
23-23: LGTM - Extension ID updated correctly.The API access point reflects the new Deepnote extension identifier.
src/test/web/index.ts (2)
47-47: LGTM - Command registration updated.
108-108: LGTM - Command invocation matches registration.src/platform/common/application/commands/runInDedicatedExtensionHost.node.ts (1)
17-17: LGTM - Command ID updated.Note: Line 12 comment still references "jupyter extension" - consider updating for consistency.
src/standalone/api/kernels/accessManagement.ts (2)
20-20: LGTM - Command ID updated.
42-43: Verify branding in UI strings.User-facing strings still reference "Jupyter Kernels" while command namespace is now
deepnote.*. Confirm whether these should also be updated to Deepnote.src/platform/common/cache.ts (1)
30-30: Verify migration strategy for renamed key.The memento key was renamed from
jupyter.jupyterServer.uriListtodeepnote.jupyterServer.uriList. Confirm whether the oldjupyter.*key should also remain in the cleanup list during the migration period to clear legacy data.src/test/datascience/interactiveDebugging.vscode.test.ts (1)
22-46: LGTM - Settings key updated consistently.All references to the IPyKernel debugger flag now use the Deepnote prefix.
.github/instructions/ipywidgets.instructions.md (3)
279-279: LGTM - Documentation updated.Comm target reference updated from
jupyter.widgettodeepnote.widget.
367-374: LGTM - Code examples updated.Documentation examples reflect the new Deepnote comm target.
743-750: LGTM - Additional examples updated.src/test/datascience/.vscode/settings.json (1)
24-39: LGTM - Test settings updated consistently.All Jupyter-prefixed settings now use the Deepnote namespace.
build/preDebugWebTest.js (2)
15-15: LGTM!File path rename is consistent with the broader Deepnote migration.
20-20: LGTM!Configuration key rename aligns with the Deepnote namespace migration.
src/notebooks/controllers/kernelSource/kernelSourceCommandHandler.ts (1)
83-210: LGTM!All kernel source command IDs consistently migrated to the
deepnote.kernel.*namespace.src/standalone/intellisense/kernelCompletionProvider.ts (2)
414-425: LGTM!Configuration checks correctly updated to the
deepnote.*namespace.
598-611: LGTM!User-facing log messages correctly reference
deepnote.completionTriggerCharacters.src/test/common.web.ts (1)
69-69: LGTM!Test command execution updated to
deepnote.selectjupyteruri.src/test/.vscode/settings.json (1)
23-23: LGTM!Test settings configuration key correctly updated to
deepnote.logging.level.src/extension.web.ts (1)
181-183: LGTM!Context keys correctly migrated to
deepnote.developmentanddeepnote.webExtension.src/platform/common/application/commands/reloadCommand.unit.test.ts (1)
22-22: LGTM!Test correctly updated to verify
deepnote.reloadVSCodecommand registration.src/kernels/errors/kernelErrorHandler.ts (1)
604-604: LGTM!Error message filtering correctly updated to check for
command:deepnote.viewOutput.src/standalone/userJupyterServer/serverSelectorForTests.ts (1)
50-50: LGTM: Command ID renamed.src/standalone/chat/extension.node.ts (1)
81-81: LGTM: Command IDs renamed.Also applies to: 105-105
src/interactive-window/editor-integration/cellRangeCache.ts (1)
64-64: LGTM: Context key reference updated.src/test/index.node.ts (1)
62-62: LGTM: Command reference in error filter updated.src/webviews/extension-side/variablesView/variableViewProvider.ts (1)
18-18: LGTM: View type rename verified and properly configured.Confirmed:
- Old ID "jupyterViewVariables" removed from package.json
- New ID "deepnoteViewVariables" properly added at line 2251 with correct webview type and when clause
src/standalone/executionAnalysis/extension.ts (1)
25-25: LGTM: Migration verified complete.All 5 command IDs consistently renamed from
jupyter.*todeepnote.*:
- extension.ts registrations updated
- package.json contributes.commands aligned
- keybindings and menus in package.json updated
- No old jupyter.* references remain
src/kernels/jupyter/interpreter/jupyterInterpreterSelectionCommand.unit.test.ts (1)
28-47: LGTM—test updates match command rename.All test references to the command consistently updated from
jupyter.selectJupyterInterpretertodeepnote.selectJupyterInterpreter.src/platform/interpreter/filter/completionProvider.node.ts (1)
71-71: LGTM—settings key updated.Configuration key renamed from
jupyter.kernels.excludePythonEnvironmentstodeepnote.kernels.excludePythonEnvironments.src/interactive-window/commands/commandRegistry.ts (1)
536-536: LGTM—view focus command updated.Command renamed from
jupyterViewVariables.focustodeepnoteViewVariables.focus.src/platform/common/application/commands/reloadCommand.node.ts (1)
16-16: LGTM—reload command ID updated.Command registration renamed from
jupyter.reloadVSCodetodeepnote.reloadVSCode.src/notebooks/controllers/commands/installPythonControllerCommands.ts (1)
63-63: LGTM—reload command invocation updated.Command execution renamed from
jupyter.reloadVSCodetodeepnote.reloadVSCode, matching the registration change.src/platform/webviews/types.ts (1)
38-38: LGTM—view type literal updated.ViewType literal renamed from
jupyterViewVariablestodeepnoteViewVariables.src/standalone/executionAnalysis/symbols.ts (1)
717-717: LGTM—code action command updated.Command renamed from
jupyter.runPrecedentCellstodeepnote.runPrecedentCells.src/notebooks/controllers/ipywidgets/message/ipyWidgetMessageDispatcher.ts (1)
95-95: LGTM—comm target updated.Pre-registered comm target renamed from
jupyter.widgettodeepnote.widget.src/extension.common.ts (1)
175-175: LGTM: Context key renamed consistently.src/test/common.node.ts (1)
241-256: LGTM: Command IDs updated consistently.src/platform/interpreter/filter/uiDeprecationHandler.ts (1)
16-19: LGTM: Command and settings key renamed consistently.src/extension.node.ts (1)
225-227: LGTM: Context keys renamed consistently.src/kernels/errors/kernelErrorHandler.unit.test.ts (3)
817-817: LGTM: Test expectation updated to new command ID.
840-840: LGTM: Test expectation updated to new command ID.
1104-1104: LGTM: Test helper updated to check new command ID.src/test/datascience/notebook/exportFull.vscode.test.ts (5)
134-134: LGTM: Export command ID renamed consistently.
167-167: LGTM: Export command ID renamed consistently.
200-200: LGTM: Export command ID renamed consistently.
252-252: LGTM: Import command ID renamed consistently.
290-290: LGTM: Import command ID renamed consistently.src/test/standardTest.node.ts (1)
147-159: LGTM: Settings keys renamed consistently.src/platform/webviews/webviewHost.ts (1)
327-327: LGTM: Configuration key renamed consistently.package.nls.json (1)
16-35: Confirm walkthrough keys match extension manifest entries manually.Walkthrough step keys were renamed from
jupyter.*todeepnote.*inpackage.nls.json, but manifest verification isn't possible via code search. Walkthrough localization doesn't require code references—only manifest registration. No stalejupyter.*patterns found in active code. Recommend checkingpackage.jsonto ensurecontributes.walkthroughssection references thedeepnotenamespace keys that now exist in the localization file.
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/common/configSettings.ts (1)
236-236: Critical: Event listener checks wrong namespace.Line 236 still checks
event.affectsConfiguration('jupyter'), but the configuration namespace was changed todeepnote. Changes todeepnote.*settings won't trigger this handler.Apply this fix:
- if (event.affectsConfiguration('jupyter')) { + if (event.affectsConfiguration('deepnote')) { onDidChange(); }
♻️ Duplicate comments (1)
src/platform/common/constants.ts (1)
300-300: Fix the typo in the context key.Line 300 contains an extra 'e' in 'interactiveeactive'. This was flagged in a previous review.
Apply this diff:
- export const IsPythonOrInteractiveOrNativeActive = 'deepnote.ispythonorinteractiveornativeeactive'; + export const IsPythonOrInteractiveOrNativeActive = 'deepnote.ispythonorinteractiveornativeactive';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (25)
src/extension.node.ts(1 hunks)src/extension.web.ts(1 hunks)src/kernels/raw/finder/trustedKernelPaths.node.ts(1 hunks)src/kernels/raw/finder/trustedKernelPaths.unit.test.ts(1 hunks)src/kernels/raw/finder/trustedKernrelPaths.unit.test.ts(1 hunks)src/kernels/raw/session/rawKernelConnection.node.ts(1 hunks)src/platform/common/configSettings.ts(2 hunks)src/platform/common/configuration/service.unit.test.ts(1 hunks)src/platform/common/constants.ts(6 hunks)src/platform/common/featureManager.ts(1 hunks)src/platform/interpreter/filter/filterService.ts(2 hunks)src/platform/interpreter/filter/settingsMigration.ts(1 hunks)src/platform/interpreter/reservedNamedProvider.node.ts(4 hunks)src/platform/interpreter/reservedNamedProvider.node.unit.test.ts(2 hunks)src/platform/logging/index.ts(2 hunks)src/standalone/executionAnalysis/extension.ts(6 hunks)src/standalone/intellisense/kernelCompletionProvider.ts(4 hunks)src/test/common.web.ts(1 hunks)src/test/datascience/interactiveWindow.vscode.common.test.ts(5 hunks)src/test/datascience/jupyter/connection.vscode.test.ts(1 hunks)src/test/datascience/notebook/exportFull.vscode.test.ts(7 hunks)src/test/datascience/notebook/intellisense/completionProvider.vscode.common.test.ts(1 hunks)src/test/datascience/widgets/standardWidgets.vscode.common.test.ts(1 hunks)src/test/datascience/widgets/thirdpartyWidgets.vscode.common.test.ts(1 hunks)src/test/web/clientApi.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/interpreter/reservedNamedProvider.node.unit.test.tssrc/platform/common/featureManager.tssrc/platform/common/configuration/service.unit.test.tssrc/platform/interpreter/filter/settingsMigration.tssrc/platform/interpreter/reservedNamedProvider.node.tssrc/platform/common/configSettings.tssrc/platform/interpreter/filter/filterService.tssrc/platform/logging/index.tssrc/platform/common/constants.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined
thenproperty to avoid hanging tests
Files:
src/platform/interpreter/reservedNamedProvider.node.unit.test.tssrc/test/datascience/widgets/thirdpartyWidgets.vscode.common.test.tssrc/kernels/raw/finder/trustedKernrelPaths.unit.test.tssrc/test/datascience/widgets/standardWidgets.vscode.common.test.tssrc/test/datascience/notebook/intellisense/completionProvider.vscode.common.test.tssrc/platform/common/configuration/service.unit.test.tssrc/test/datascience/jupyter/connection.vscode.test.tssrc/test/datascience/interactiveWindow.vscode.common.test.tssrc/kernels/raw/finder/trustedKernelPaths.unit.test.tssrc/test/datascience/notebook/exportFull.vscode.test.ts
src/kernels/**/finder/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/finder/**/*.ts: All finder implementations must implement the IContributedKernelFinder interface
Finder implementations should use caching to avoid redundant discovery work
Activate finder scans lazily (only when needed) to reduce startup cost
Files:
src/kernels/raw/finder/trustedKernrelPaths.unit.test.tssrc/kernels/raw/finder/trustedKernelPaths.node.tssrc/kernels/raw/finder/trustedKernelPaths.unit.test.ts
src/kernels/raw/finder/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Raw kernel discovery must scan local kernel specs and Python environments
Files:
src/kernels/raw/finder/trustedKernrelPaths.unit.test.tssrc/kernels/raw/finder/trustedKernelPaths.node.tssrc/kernels/raw/finder/trustedKernelPaths.unit.test.ts
src/kernels/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations
Files:
src/kernels/raw/finder/trustedKernrelPaths.unit.test.tssrc/kernels/raw/finder/trustedKernelPaths.node.tssrc/kernels/raw/session/rawKernelConnection.node.tssrc/kernels/raw/finder/trustedKernelPaths.unit.test.ts
src/kernels/{jupyter,raw}/session/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
All session implementations must conform to IKernelSession
Files:
src/kernels/raw/session/rawKernelConnection.node.ts
src/kernels/raw/session/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
Raw sessions must use ZeroMQ socket communication
Files:
src/kernels/raw/session/rawKernelConnection.node.ts
src/platform/**/*.node.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.node.ts: Provide Node.js-specific implementations in .node.ts files (e.g., FileSystem with fs-extra, PlatformService with OS detection)
Desktop implementations may use Node capabilities (fs-extra, process spawning, native modules) consistent with their responsibilities
Files:
src/platform/interpreter/reservedNamedProvider.node.ts
src/platform/logging/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Files:
src/platform/logging/index.ts
🧬 Code graph analysis (10)
src/platform/interpreter/reservedNamedProvider.node.unit.test.ts (2)
src/test/vscode-mock.ts (1)
mockedVSCodeNamespaces(17-17)src/platform/interpreter/reservedNamedProvider.node.ts (1)
ignoreListSettingName(16-16)
src/test/datascience/widgets/thirdpartyWidgets.vscode.common.test.ts (1)
.vscode-test.mjs (1)
config(18-41)
src/kernels/raw/finder/trustedKernrelPaths.unit.test.ts (1)
src/test/vscode-mock.ts (1)
mockedVSCodeNamespaces(17-17)
src/test/web/clientApi.ts (1)
src/test/datascience/ipywidgets/samples/beakerx/extension.js (1)
workspace(21144-21144)
src/test/common.web.ts (2)
build/preDebugWebTest.js (1)
url(11-11)src/test/constants.ts (1)
JUPYTER_SERVER_URI(29-29)
src/platform/common/configuration/service.unit.test.ts (2)
src/test/vscode-mock.ts (1)
mockedVSCodeNamespaces(17-17)src/test/datascience/helpers.ts (1)
uriEquals(328-338)
src/extension.node.ts (2)
src/platform/common/types.ts (1)
IsDevMode(9-9)src/platform/common/utils/misc.ts (1)
noop(11-11)
src/extension.web.ts (2)
src/platform/common/types.ts (1)
IsDevMode(9-9)src/platform/common/utils/misc.ts (1)
noop(11-11)
src/standalone/intellisense/kernelCompletionProvider.ts (2)
src/kernels/types.ts (1)
IKernel(439-446)src/kernels/helpers.ts (1)
getDisplayNameOrNameOfKernelConnection(250-317)
src/kernels/raw/finder/trustedKernelPaths.unit.test.ts (1)
src/test/vscode-mock.ts (1)
mockedVSCodeNamespaces(17-17)
🔇 Additional comments (46)
src/test/datascience/interactiveWindow.vscode.common.test.ts (6)
81-82: LGTM! Configuration namespace correctly updated.The change from
'jupyter'to'deepnote'is consistent with the PR's renaming objective.
136-136: LGTM! Command ID correctly updated.
143-143: LGTM! Restart kernel command correctly updated.
222-223: LGTM! Configuration namespace correctly updated.
400-401: LGTM! Configuration namespace correctly updated.
579-580: LGTM! Configuration namespace correctly updated.src/test/datascience/widgets/standardWidgets.vscode.common.test.ts (1)
119-119: Configuration namespace updated correctly.The change from
'jupyter'to'deepnote'aligns with the PR's renaming objective.src/test/datascience/widgets/thirdpartyWidgets.vscode.common.test.ts (1)
58-58: Configuration namespace updated correctly.Consistent with the namespace migration from
'jupyter'to'deepnote'.src/test/common.web.ts (1)
69-69: LGTM.Command rename is consistent with the namespace migration.
src/kernels/raw/finder/trustedKernrelPaths.unit.test.ts (1)
24-26: LGTM: Test mock updated to Deepnote namespace.Configuration mock correctly references the new namespace.
src/platform/interpreter/filter/settingsMigration.ts (1)
19-45: LGTM: Configuration namespace migration applied consistently.All
getConfigurationcalls now use the Deepnote namespace across all configuration targets (Global, Workspace, WorkspaceFolder).src/kernels/raw/session/rawKernelConnection.node.ts (1)
683-687: LGTM: Logging flag now reads from Deepnote configuration.The conditional Python kernel logging correctly uses the new namespace.
src/platform/interpreter/filter/filterService.ts (3)
29-29: LGTM: Configuration change listener updated to Deepnote namespace.Now consistent with the configuration reads below.
58-59: LGTM: Configuration read updated to Deepnote namespace.Aligns with the change listener at line 29.
65-66: LGTM: Per-folder configuration read updated to Deepnote namespace.Consistent with other configuration accesses in this file.
src/platform/interpreter/reservedNamedProvider.node.unit.test.ts (2)
39-40: LGTM: Test configuration mock updated to Deepnote namespace.Mock setup aligns with the namespace migration.
200-201: LGTM: Configuration change test updated to Deepnote namespace.Test assertion correctly verifies the new configuration section.
src/kernels/raw/finder/trustedKernelPaths.node.ts (1)
19-20: LGTM: Trusted kernel paths now read from Deepnote configuration.Configuration access updated to the new namespace.
src/test/datascience/jupyter/connection.vscode.test.ts (1)
231-247: LGTM: Test configuration read updated to Deepnote namespace.Connection test now correctly reads from the Deepnote configuration section.
src/extension.web.ts (2)
177-177: LGTM: Development flag now reads from Deepnote configuration.Extension mode detection updated to the new namespace.
181-183: LGTM: VS Code context keys updated to Deepnote namespace.Context keys for development mode and web extension now use the Deepnote prefix.
src/kernels/raw/finder/trustedKernelPaths.unit.test.ts (1)
25-27: LGTM - test mock updated consistently.Mock now retrieves configuration from the
deepnotenamespace, aligning with the broader namespace migration.src/test/datascience/notebook/intellisense/completionProvider.vscode.common.test.ts (1)
46-52: LGTM - configuration namespace migrated.Test setup now reads from
deepnotenamespace consistently with the PR's rename objective.src/platform/common/configuration/service.unit.test.ts (1)
26-28: LGTM - test helper updated.Mock setup now uses
deepnotenamespace, consistent with the migration.src/platform/common/configSettings.ts (2)
225-227: LGTM - configuration read updated.Correctly reads from
deepnotenamespace in the change handler.
245-250: LGTM - initialization updated.Correctly reads from
deepnotenamespace during initialization.src/platform/common/featureManager.ts (2)
130-132: LGTM - multi-root workspace check updated.Correctly reads from
deepnotenamespace when checking deprecated settings across workspace folders.
136-138: LGTM - single-workspace check updated.Correctly reads from
deepnotenamespace for single-workspace deprecated setting checks.src/test/web/clientApi.ts (1)
13-13: LGTM - configuration read updated.Correctly reads from
deepnotenamespace.src/extension.node.ts (1)
219-227: LGTM - development mode and context keys migrated.Configuration read and VS Code context keys consistently updated to
deepnotenamespace.src/test/datascience/notebook/exportFull.vscode.test.ts (9)
112-113: LGTM - teardown configuration updated.Correctly reads from
deepnotenamespace.
134-134: LGTM - export command migrated.Command ID updated to
deepnote.exportAsPythonScript.
163-164: LGTM - configuration and export method updated.Correctly uses
deepnotenamespace for settings.
167-167: LGTM - export command migrated.Command ID updated to
deepnote.exportAsPythonScript.
196-197: LGTM - configuration updated.Correctly uses
deepnotenamespace.
200-200: LGTM - export command migrated.Command ID updated to
deepnote.exportAsPythonScript.
252-252: LGTM - import command migrated.Command ID updated to
deepnote.importnotebookfile.
277-278: LGTM - configuration updated.Correctly uses
deepnotenamespace.
290-290: LGTM - import command migrated.Command ID updated to
deepnote.importnotebookfile.src/standalone/executionAnalysis/extension.ts (1)
10-78: LGTM!Configuration and command ID renames are consistent and complete.
src/platform/interpreter/reservedNamedProvider.node.ts (1)
15-121: LGTM!Memento key and configuration namespace migrations are correct and complete.
src/standalone/intellisense/kernelCompletionProvider.ts (1)
414-606: LGTM!Configuration namespace and user-facing log messages correctly updated.
src/platform/common/constants.ts (3)
45-82: LGTM!Comment and DefaultCommTarget correctly renamed.
173-299: LGTM!Command IDs and editor context keys correctly migrated to deepnote namespace.
301-527: LGTM!Remaining context keys and widget MIME types correctly migrated.
src/platform/logging/index.ts (1)
61-61: Configuration namespace now consistent.The previous mismatch between the change listener and config reader is resolved—both now use
deepnote.
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/platform/logging/index.ts (1)
66-66: OutputChannelNames.jupyter still references old namespace.Configuration, logger label, and change listener all use 'deepnote', but the output channel still uses
OutputChannelNames.jupyter. Update to align with the migration.Run the following to verify if OutputChannelNames.deepnote exists:
#!/bin/bash # Check OutputChannelNames definition and available properties ast-grep --pattern 'OutputChannelNames = { $$$ }' # Check all references to OutputChannelNames in the codebase rg -n 'OutputChannelNames\.(jupyter|deepnote)' --type ts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/platform/logging/index.ts(3 hunks)src/test/web/clientApi.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/logging/index.ts
src/platform/logging/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
Implement logging using OutputChannelLogger/ConsoleLogger and honor LogLevel; ensure PII scrubbing in log output
Files:
src/platform/logging/index.ts
🧬 Code graph analysis (2)
src/platform/logging/index.ts (2)
src/platform/logging/consoleLogger.ts (1)
ConsoleLogger(14-39)src/platform/common/constants.ts (1)
isCI(50-50)
src/test/web/clientApi.ts (1)
src/test/datascience/ipywidgets/samples/beakerx/extension.js (1)
workspace(21144-21144)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (1)
src/test/web/clientApi.ts (1)
10-12: Previous interpolation issue resolved.The template literal now correctly uses
${}syntax.
Summary by CodeRabbit